Fix #11654: create new symbol for stdlib patches #11803
Conversation
liufengyun
force-pushed
the
fix-11654
branch
2 times, most recently
from
d4b9537 to
a33db77
Compare
liufengyun
force-pushed
the
fix-11654
branch
2 times, most recently
from
224a0da to
6219910
Compare
| patch.denot = patch.denot.copySymDenotation(owner = denot.symbol) | ||
| patch | ||
| else | ||
| // change `info` which might contain reference to the patch |
There was a problem hiding this comment.
Could we use a TreeTypeMap to map the info instead?
There was a problem hiding this comment.
You mean for the inline tree in the annotation? It's quite complex -- we need to create a copy of all local symbols (e.g. for type, term symbols, symbols in the function body, etc.) and rewire them.
For non-inline symbols, we make strong assumptions on symbol.info, which seems to be simpler.
There was a problem hiding this comment.
I meant for both, but I don't know if TreeTypeMap works on inline defs
There was a problem hiding this comment.
I don't see a principled way to do this --- a map needs to make assumptions about from and to. We will not make fewer assumptions and the code will be complex.
| def recurse(patch: Symbol) = patch.is(Module) && scope.lookup(patch.name).exists | ||
|
|
||
| def makeClassSymbol(patch: Symbol, parents: List[Type], selfInfo: TypeOrSymbol) = | ||
| newClassSymbol( |
There was a problem hiding this comment.
Why can't we use patch.copy(...) here? Would that not be shorter?
There was a problem hiding this comment.
The reason is that info and symbol depend on each other, and Symbol.copy does not support that.
| name = patch.name.asTypeName, | ||
| flags = patch.flags, | ||
| // need to rebuild a fresh ClassInfo | ||
| infoFn = cls => ClassInfo( |
There was a problem hiding this comment.
Would it not be shorter to use derivedClassInfo here?
There was a problem hiding this comment.
That does not make much difference, as we change all fields here except parents. On the other hand, using the constructor makes the implicit assumptions explicit and clear here.
Fix #11654: create new symbol for stdlib patches
[test_windows_full]